Skip to content

Fix timezone issue when calculating min/max values in tibber #14009#14080

Merged
MartinHjelmare merged 2 commits intodevfrom
#14009
Apr 26, 2018
Merged

Fix timezone issue when calculating min/max values in tibber #14009#14080
MartinHjelmare merged 2 commits intodevfrom
#14009

Conversation

@Danielhiversen
Copy link
Copy Markdown
Member

Description:

fix timezone issue when calculating min/max values in tibber #14009

Related issue (if applicable): fixes #14009

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

@Danielhiversen Danielhiversen added this to the 0.68 milestone Apr 25, 2018
if (not self._newest_data_timestamp or
(self._newest_data_timestamp - now).total_seconds()/3600 < 12
if (not self._last_data_timestamp or
(self._last_data_timestamp - now).total_seconds()/3600 < 12
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the check < 12? That looks like the opposite of what I would expect.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data change every hour, but is available 12-36 hours ahead. So when we have less then 12 hours of future data available new data should be available. We do not know exactly when the new is available, so we might need to retry.

now = dt_util.now()
if self._tibber_home.current_price_total and self._last_updated and \
self._last_updated.hour == now.hour and self._newest_data_timestamp:
self._last_updated.hour == now.hour and self._last_data_timestamp:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why shouldn't we let the scan interval decide on the update frequency?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data change every hour, but we want to retry if no data are available.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the data hasn't changed since last update, making a state update will not write a new state to the state machine. So you could just try to do that without making the check here. Are there any downsides, you think? It would make the code cleaner.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understood your last comment.
But if I do not return here, the _update_current_price will be called every minute

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and if all values including state and state attributes remain the same a call to do a state update, will not write a new state to the state machine, so will not clog the database.

The only thing that seems to change, while fetched data is the same, is the time_diff variable that is computed from the current time of the call. What will happen there?

Copy link
Copy Markdown
Member Author

@Danielhiversen Danielhiversen Apr 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there will be no state change and no new state to the state machine.
But I though it would be good avoid calling _update_current_price each minute, and then also avoid iterate over all prices each minute.
So the return here is only to avoid unnecessary cpu load.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. My comment about possible clean up, if makes sense, is for another PR.

@MartinHjelmare MartinHjelmare merged commit d7f7735 into dev Apr 26, 2018
@Danielhiversen Danielhiversen deleted the #14009 branch April 26, 2018 07:52
balloob pushed a commit that referenced this pull request Apr 27, 2018
@balloob balloob mentioned this pull request Apr 27, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Sep 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tibber reports wrong values

4 participants